Conversation
SampleSpectrum generates data for intensity values. Previously, it was lagging when 5000 points generated every second. Vectorizing SampleSpectrum to generate all 5000 points in single batch imroved performance considerably.
…ample-apps into dashr-ocean-optics
rpkyle
left a comment
rpkyle
left a comment
There was a problem hiding this comment.
@CanerIrfanoglu Looks good so far, a few minor edits requested before we merge this one. Thanks for taking this on! Let me know if you have any questions or want to discuss.
| ), | ||
|
|
||
| function(pwr, ls) { | ||
| return(!(pwr && ls != "NULL" && ls != "")) |
There was a problem hiding this comment.
Does this logical expression evaluate as intended? I might be missing something, but generally in R we test for NULL by using something like is.null(foo), since NULL is a special object rather than a character string.
I would also rename ls, since this is a base R function.
It's unfortunate that c in R is also a function, since it looks like a variable name in most other languages 🙁
There was a problem hiding this comment.
Changed to:
function(pwr, lsi) {
return(!(pwr && !is.null(lsi[[1]]) && lsi != ""))
}
)
Since lsi is type list when the dccDropdown is empty and character otherwise, had to change lsi to lsi[[1]] to ensure capturing boolean value in empty scenario.
rpkyle
left a comment
rpkyle
left a comment
There was a problem hiding this comment.
Looking good! I think it's ready to go, except for one last detail: please replace the logo with the current one (which I think you're already using in the IV tracer app) 🙂 Thanks, and nice work! 💃
sacul-git
left a comment
sacul-git
left a comment
There was a problem hiding this comment.
💃 Looks good! Didn't find much to improve on :)
| } | ||
|
|
||
| # colors for layout and figure | ||
| colorsRaw <- read.table("colors.txt", sep = ' ', comment.char = '', |
There was a problem hiding this comment.
Not an important comment, but just out of curiosity, is there a reason to store the color data in a separate file? Why not just use a list like:
colors <- list(
background = "#bbbbbb",
primary = "#ffffff",
secondary = "#efefef",
tertiary = "#dfdfdf",
grid-colour = "#eeeeee",
accent = "#2222ff"
)
There was a problem hiding this comment.
Since Python version was using a separate colors.txt, I did it the same way. Also, I wouldn't mind defining it as a list in app.R :)
3 participants
App pull request
About
Workflow
elements necessary for successful deployment are in place.
existing gallery app, I've summarized the changes requested in the
appropriate Streambed issue and confirm that they have been applied.
the GitHub repository for the source code in the portal description.
DashR gallery, the app in this PR mimics, as closely as possible,
the style and functionality of the existing app.=
assets/folder.The pre-review review
I have addressed all of the following questions:
any dead and/or irrelevant code.)
readable and, where it isn't, it has been commented appropriately.)]
lessen the volume of code that needs to be maintained.)